feat: assignment and bandit caching #82
Conversation
| @@ -63,6 +81,12 @@ public static class Builder { | |||
| private long pollingIntervalMs = DEFAULT_POLLING_INTERVAL_MS; | |||
| private String host = DEFAULT_HOST; | |||
|
|
|||
| // Assignment and bandit caching on by default. To disable, call | |||
| // `builder.assignmentCache(null).banditAssignmentCache(null);` | |||
| private IAssignmentCache assignmentCache = new LRUInMemoryAssignmentCache(100); | |||
There was a problem hiding this comment.
How much of this configuration should we surface to devs?
The cache size or ttl? or the IAssignmentCache. Or should we surface as a set of caching options on the builder, similar to JS (ex: withLruAssignmentCaching(100), withExpiringCache(10, MINUTES) etc)
There was a problem hiding this comment.
I like the option of providing convenience methods like withLruAssignmentCaching and withExpiringCache, however we should also allow users to bring their own implementation of IAssignmentCache, so we also need a withAssignmentCache that takes the interface. EDIT: Looks like you already added it below, so we're good on that one
build.gradle
Outdated
| @@ -31,7 +31,7 @@ repositories { | |||
|
|
|||
| dependencies { | |||
| // Re-export classes and interfaces that will be used upstream | |||
| api 'cloud.eppo:sdk-common-jvm:3.3.1' | |||
| api 'cloud.eppo:sdk-common-jvm:3.5.0-SNAPSHOT' | |||
There was a problem hiding this comment.
you probably don't need this to be a snapshot version anymore since v3.5.0 has been released, right? same below line 43
| boolean isGracefulModel) { | ||
| boolean isGracefulModel, | ||
| IAssignmentCache assignmentCache, | ||
| IAssignmentCache banditAssignmentCache) { |
There was a problem hiding this comment.
nit: Add @Nullable or @NonNull annotations as needed
| @@ -63,6 +81,12 @@ public static class Builder { | |||
| private long pollingIntervalMs = DEFAULT_POLLING_INTERVAL_MS; | |||
| private String host = DEFAULT_HOST; | |||
|
|
|||
| // Assignment and bandit caching on by default. To disable, call | |||
| // `builder.assignmentCache(null).banditAssignmentCache(null);` | |||
| private IAssignmentCache assignmentCache = new LRUInMemoryAssignmentCache(100); | |||
There was a problem hiding this comment.
I like the option of providing convenience methods like withLruAssignmentCaching and withExpiringCache, however we should also allow users to bring their own implementation of IAssignmentCache, so we also need a withAssignmentCache that takes the interface. EDIT: Looks like you already added it below, so we're good on that one
Available by upgrading to the latest version of the common SDK